feat: 🎸 move pricing agent to use a safer delete query#877
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Additional Comments (1)
The PR title is "move pricing agent to use a safer delete query," but the pricing agent handler ( Prompt To Fix With AIThis is a comment left during a code review.
Path: server/src/internal/misc/pricingAgent/handlers/handleSyncPreviewPricing.ts
Line: 74-78
Comment:
**Pricing agent not updated as stated in PR title**
The PR title is "move pricing agent to use a safer delete query," but the pricing agent handler (`handleSyncPreviewPricing.ts`) still calls `CusService.deleteByOrgId` on line 74 — the original unbatched method that locks all rows at once. Only `handleNukeOrganisationConfiguration.ts` was updated to use `safeDeleteByOrgId`. If the intent is to protect the pricing agent from table-lock issues, this file needs the same fix.
```suggestion
await CusService.safeDeleteByOrgId({
db,
orgId: previewOrg.id,
env: AppEnv.Sandbox,
});
```
How can I resolve this? If you propose a fix, please make it concise. |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Switch org-wide deletes to batched queries for customers, features, and products to avoid locks/timeouts when nuking sandbox orgs. Customer deletes remain blocked in Live, and the delete for customers now also scopes by org_id and env.
Written for commit a214068. Summary will update on new commits.
Greptile Summary
This PR introduces
CusService.safeDeleteByOrgId, a batched alternative to the existingdeleteByOrgIdthat selects and deletes customers 250 rows at a time to avoid acquiring a full table lock during large deletions. ThehandleNukeOrganisationConfigurationhandler is updated to use the new safe method.Key changes:
CusService.safeDeleteByOrgId— batched (default 250) customer deletion scoped to a non-live environment, preventing long-held table locks during sandbox cleanup.handleNukeOrganisationConfigurationnow usessafeDeleteByOrgIdinstead of the single-querydeleteByOrgId.handleSyncPreviewPricing) was also updated, but it still calls the originalCusService.deleteByOrgId— this appears to be an incomplete change and the same fix should be applied there.Confidence Score: 3/5
safeDeleteByOrgIdimplementation is logically sound and the live-mode guard is preserved. However, the PR title explicitly states the goal is to move the pricing agent to the safer delete, yethandleSyncPreviewPricing.tsstill calls the olddeleteByOrgId. This is an incomplete implementation of the stated intent, dropping the score to 3.deleteByOrgIdand should be updated tosafeDeleteByOrgId.Important Files Changed
safeDeleteByOrgIdwhich deletes customers in batches (default 250) to avoid table-wide locks; guarded against live mode. Minor: the inner delete statement lacksorg_id/envfilters as a defensive measure.deleteByOrgIdforsafeDeleteByOrgIdin the nuke-organisation handler; no issues in this file itself.Sequence Diagram
sequenceDiagram participant Handler as handleNukeOrganisationConfiguration participant CusService participant DB as Database (customers table) Handler->>CusService: safeDeleteByOrgId(db, orgId, env=Sandbox) CusService->>CusService: guard: throw if env == Live loop Until no rows remain CusService->>DB: SELECT internal_id WHERE org_id=orgId AND env=Sandbox LIMIT 250 DB-->>CusService: batch of internal_ids (up to 250) alt batch is empty CusService-->>Handler: done (break) else batch has rows CusService->>DB: DELETE WHERE internal_id IN (batch) DB-->>CusService: rows deleted end endLast reviewed commit: fc5ff77